Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Daemon.jsonrpc_get: add download_collection option for handling collections #3415

Closed
wants to merge 1 commit into from

Conversation

belikor
Copy link
Contributor

@belikor belikor commented Sep 4, 2021

This can be combined with #3411, but in this pull request we only show the code to handle collections.

Normally, if a particular claim is in fact a collection (playlist), it will not be downloaded, as it is not a stream.

lbrynet get collection-claim

Will result in

{
  "error": "Claim is not a stream."
}

To resolve the collection, and download the claims that it contains we can use a new option --download_collection.

lbrynet get collection-claim --download_collection

A collection can have an arbitrary number of items, so to limit the number of claims to download we can use --max_claims.

lbrynet get collection-claim --download_collection --max_claims=4

To reverse the order of the claims in the collection use --reverse_collection. This can be used to get the newest items in the collection.

lbrynet get collection-claim --download_collection --max_claims=10 --reverse_collection

The inner code to download a URL is put in its own method _get, then the collection is parsed, and _get is used on each of the internal claims.


After merging #3411, we can refactor this pull request to be able to use a claim ID.

lbrynet get collection-music
lbrynet get collection-music --download_collection
lbrynet get collection-music --download_collection --max_claims=1 --reverse_collection

lbrynet get 87560bfa01c2fb92d39394e4d508cf7a453eef77 --claim_id
lbrynet get 87560bfa01c2fb92d39394e4d508cf7a453eef77 --claim_id --download_collection
lbrynet get 87560bfa01c2fb92d39394e4d508cf7a453eef77 --claim_id --download_collection --max_claims=1 --reverse_collection

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 67.984% when pulling c57acfd on belikor:get-collection into 72049af on lbryio:master.

@eukreign
Copy link
Member

eukreign commented Sep 7, 2021

@lyoshenka please review the API changes

@eukreign eukreign assigned lyoshenka and unassigned eukreign Sep 7, 2021
…lections

Normally, if a particular claim is in fact a collection (playlist),
it will not be downloaded, as it is not a stream.
```
lbrynet get collection-claim
```

Will result in
```
{
  "error": "Claim is not a stream."
}
```

To resolve the collection, and download the claims that it contains
we can use the option `--download_collection`.
```
lbrynet get collection-claim --download_collection
```

A collection can have an arbitrary number of items, so to limit
the number of claims to download we can use `--max_claims`.
```
lbrynet get collection-claim --download_collection --max_claims=4
```

To reverse the order of the claims in the collection
use `--reverse_collection`. This can be used to get the newest
items in the collection.
```
lbrynet get collection-claim --download_collection --max_claims=10 --reverse_collection
```
Copy link
Member

@lyoshenka lyoshenka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need for a --download_collection flag. If you call get on a collection, then you want to download the collection.

I'd rename --max_claims to --collection_limit so it's clear that flag applies to collections.

Otherwise the API looks good to me. @eukreign please review the code itself once @belikor makes these changes.

@lbry-bot lbry-bot assigned eukreign and unassigned lyoshenka Oct 4, 2021
@lyoshenka lyoshenka assigned belikor and unassigned eukreign Oct 4, 2021
@kauffj
Copy link
Member

kauffj commented Oct 11, 2021

@belikor planning to come back to this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants